feat: add integration test infrastructure with E2E SMS tests#879
feat: add integration test infrastructure with E2E SMS tests#879AchoArnold merged 4 commits intomainfrom
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Greptile SummaryThis PR adds an integration test infrastructure with a Go-based phone emulator, Docker Compose stack, and two E2E tests (send and receive SMS) that run in CI before deployment.
Confidence Score: 3/5Not safe to merge as-is — the Go version mismatch will break CI and the RoundTripper mutation needs fixing. Two P1 findings: the go.mod/CI toolchain mismatch will cause the integration test step to fail on every run, and the RoundTripper contract violation can produce data races. These both need to be resolved before the PR can function as intended. tests/go.mod (go version), api/pkg/di/fcm_transport.go (request mutation), tests/firebase-credentials.json (committed key material)
|
| Filename | Overview |
|---|---|
| api/pkg/di/fcm_transport.go | New RoundTripper that redirects FCM traffic to the emulator; mutates the request URL in place, violating the http.RoundTripper contract and risking data races. |
| tests/go.mod | Declares go 1.25.0 while CI installs Go 1.22; toolchain mismatch will cause the integration test step to fail or require an unplanned toolchain download. |
| tests/firebase-credentials.json | Fake service-account JSON with an embedded RSA private key; will trigger secret-scanning tools despite being test-only. |
| tests/integration_test.go | E2E tests for SMS send and receive; uses bare type assertions that will panic on unexpected API responses instead of failing cleanly. |
| tests/docker-compose.yml | Compose stack for integration tests; redis:latest is unpinned and could introduce instability across runs. |
| .github/workflows/integration-test.yml | New CI workflow that builds the stack, seeds data, and runs E2E tests before deployment; go-version: "1.22" conflicts with tests/go.mod requiring 1.25.0. |
| tests/helpers_test.go | Test helpers including a polling helper for message status; implementation is clean. |
| tests/seed.sql | Seeds test user, system user, phone, and phone API key using idempotent ON CONFLICT DO NOTHING; looks correct. |
Sequence Diagram
sequenceDiagram
participant CI as GitHub Actions CI
participant API as API (Docker)
participant Emulator as Phone Emulator
participant DB as PostgreSQL
CI->>API: docker compose up (build + wait)
CI->>DB: seed.sql (users, phone, api_key)
CI->>API: POST /v1/messages/send (userAPIKey)
API->>Emulator: FCM push (via fcmRedirectTransport)
Emulator-->>API: 200 fake-message-id
Emulator->>API: GET /v1/messages/outstanding (phoneAPIKey)
API-->>Emulator: [pending messages]
Emulator->>API: POST /v1/messages/{id}/events (SENT)
Emulator->>API: POST /v1/messages/{id}/events (DELIVERED)
CI->>API: GET /v1/messages/{id} poll until delivered
API-->>CI: 200 status=delivered
CI->>API: POST /v1/messages/receive (phoneAPIKey)
API-->>CI: 200 status=received
CI->>API: GET /v1/messages/{id}
API-->>CI: 200 status=received
Reviews (1): Last reviewed commit: "ci: add integration test workflow" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func (t *fcmRedirectTransport) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| req.URL.Scheme = t.target.Scheme | ||
| req.URL.Host = t.target.Host | ||
| return t.base.RoundTrip(req) |
There was a problem hiding this comment.
The http.RoundTripper contract explicitly states that an implementation must not modify the request (only consume/close the Body). Directly writing to req.URL.Scheme and req.URL.Host mutates the caller's *http.Request, which can cause data races when the Firebase SDK reuses or retries the same request. The URL should be cloned before modification.
| func (t *fcmRedirectTransport) RoundTrip(req *http.Request) (*http.Response, error) { | |
| req.URL.Scheme = t.target.Scheme | |
| req.URL.Host = t.target.Host | |
| return t.base.RoundTrip(req) | |
| func (t *fcmRedirectTransport) RoundTrip(req *http.Request) (*http.Response, error) { | |
| cloned := req.Clone(req.Context()) | |
| cloned.URL.Scheme = t.target.Scheme | |
| cloned.URL.Host = t.target.Host | |
| return t.base.RoundTrip(cloned) | |
| } |
| @@ -0,0 +1,11 @@ | |||
| module github.com/NdoleStudio/httpsms/tests | |||
|
|
|||
| go 1.25.0 | |||
There was a problem hiding this comment.
tests/go.mod declares go 1.25.0 but the CI workflow installs go-version: "1.22". With Go 1.21+ toolchain management, running go test ./... under Go 1.22 on a module requiring 1.25.0 will cause the toolchain to attempt to download Go 1.25 (GOTOOLCHAIN=auto) or fail outright (GOTOOLCHAIN=local). The emulator's go.mod correctly uses go 1.22; tests/go.mod should match or the CI should pin a version that satisfies this directive.
| go 1.25.0 | |
| go 1.22 |
| { | ||
| "type": "service_account", | ||
| "project_id": "httpsms-test", | ||
| "private_key_id": "test-key-id", | ||
| "private_key": "-----BEGIN RSA PRIVATE KEY-----\\nMIIEogIBAAKCAQEAqx29E5Eea63wVdoWUiFetrCw0B/wpsi2/mD6eHmkeue6n0mS\\noVpw5Mb11z/iiDUDL1eX6b3oPdCruaKfvnJ4t2J2yXAT6hoPy+RiWJDRUZazHyKH\\nx68YkRDI1xMhFpn8CbC16vW9DGz2mSdEwI3M8VDxw0UDcCgB3U7WdsBuIaXS6Vs0\\n9pohYuRY3yk+CC6Q5MBh5RlRpO4xOQ2JHNJkMkG+av952Uro7J8/b1cnH3dMRpbO\\nDXtXgq0bSjuJVbd8c9ymDHcl7A5uAKdu8HCd5xbYVp9yKTzwanl2Kxx3Dphp8YaL\\nmJ1k5bgXaHJ2fhx7ESw2gv5lhTbxuMtd4esqdQIDAQABAoIBAHOqqoBre/C1ptuh\\ni60AuZEsZpiIvpc+3dOdojGFqFUcBt5dUSyYge9jPhK+MFZ53ylFQH7TzATc5Pea\\nofiOUGNFv53ykMOR0lO0kXXkjllkULgfE0E7bpPAkMIxQBCTDfdO5+lnKt8XWKm2\\nDZdLQtlsKcAhCm3p3TjHbdjfwpIi9VTV7bDoS/XbNi/vDKrL5KaH8w8zmg3M5VPV\\nuXwM5JempH4IfF0O57ZaZPjgeGgeSt3ZBBha8gD5xsVbnrpN0uIa8HBfx65sK9kX\\ny5mD9yBezvRZTYvNIZAajGl5kY/G0sOMSJZ+K6CwRxUT11Cnph9F2cPzX40vszY9\\noN1CEpECgYEAwXB5OFQ832xV8IS2zgztJHb6Jb+jdhFbz8uCy7rpc99L1m8XdprA\\nYAgjfXVQcoQrhxWKRN+RiWzNb2iNBqMujP1ebUi212JGa5QT5zX7YkExXOyL0b1U\\nead7hu+zox8MXi5cnxn/OWHpCV5VuSLxe1X/DRxl3/yQNWtthC3rSfMCgYEA4nUM\\nyA3Wj5I9zhCWSRdWUqQ0/lF6tGNiAXMeQLxGvHlESes8EYssCSDBaFE7MUlEllwL\\nNQJetSr0uxK9xBoiOmEfyCDtT8YQ/nDmoAPIGNWv84xsm+JzN1OfZAJxP1IhfmBl\\n+eKk6EeSQ213oEZBODsSksWvUOW9gGw+WYNZK/cCgYAi0rKf11pWBlS6Rcn68gzG\\n4bxKi2NL3/gdQk+7iVx5hZtLcRhSO62iHBT3guUGmJCVcuU/XNgAW1voUHQC2+GH\\ndB1Joyt3PrParf+fQfKNT5spVVQeX+0TMXllY0V9ehtp4QK/iZiOpfejvh3EhGvg\\nfiy6GYClf/weAcbnYrTWzQKBgEDnKS2hAbCOSlZn3JrALIZs9583/QH8RdeChdYp\\n3+AepVrGJ6YjjBBlqeja9ysOA7FQWgnsTvZ/hDqliWvNzaeLtI8oGLu3WaGC/CY6\\nTzwG6nHT+kDHKxxdRB5msMxkHqNYv4FZ2seT07CtjA6MdB7Il28nZzK5VZGb9Fa1\\nVXZLAoGAJRTsLzFyjlj/yCEhDLjeawRKbaVCQp5Qvb+wCqLpH1Q4u9tDg5e7A0H3\\nOXOUNUAOv7ufpofSWzKnujvjEURhScKnxiuAz/7a0Cd9Cen3ZK3vyaUr1AcvqNOE\\nCygZOEMRktGpP9zFX+RZV5jqGBEMkFft+IZ1bRwyUYsxKxTByqc=\\n-----END RSA PRIVATE KEY-----\\n", | ||
| "client_email": "test@httpsms-test.iam.gserviceaccount.com", | ||
| "client_id": "123456789", | ||
| "auth_uri": "http://emulator:9090/auth", | ||
| "token_uri": "http://emulator:9090/token", | ||
| "auth_provider_x509_cert_url": "http://emulator:9090/certs", | ||
| "client_x509_cert_url": "http://emulator:9090/certs/test" | ||
| } |
There was a problem hiding this comment.
This file embeds what appears to be an RSA private key (-----BEGIN RSA PRIVATE KEY-----). Even though it is a fake/test credential, committing it will immediately trigger secret-scanning tools (GitHub Advanced Security, truffleHog, etc.) and may block merges or generate alerts. A better approach is to generate this JSON dynamically in the workflow (e.g., using a jq one-liner) or move the key material into a GitHub Actions secret.
| data := sendResult["data"].(map[string]interface{}) | ||
| messageID := data["id"].(string) |
There was a problem hiding this comment.
sendResult["data"].(map[string]interface{}) and data["id"].(string) are bare type assertions. If the API returns an error body (e.g., a "data": null response), these will panic with a non-descriptive runtime error rather than failing the test cleanly. The same pattern appears on lines 69-70 and 85. Prefer the two-value form v, ok := ...; require.True(t, ok) to get a readable failure message.
| start_period: 5s | ||
|
|
||
| redis: | ||
| image: redis:latest |
There was a problem hiding this comment.
redis:latest image
Using redis:latest means a breaking Redis major version upgrade can silently change the test environment and cause non-deterministic failures. Pin to a specific minor version (e.g., redis:7-alpine) to control upgrades explicitly.
| image: redis:latest | |
| image: redis:7-alpine |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 1 medium |
| BestPractice | 3 medium |
| Security | 1 minor 13 high 1 critical 26 medium |
🟢 Metrics 59 complexity · 10 duplication
Metric Results Complexity 59 Duplication 10
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
AchoArnold
force-pushed
the
feature/integration-tests
branch
from
e9f83ea to
370d1c0
Compare
AchoArnold
force-pushed
the
feature/integration-tests
branch
from
5fb2945 to
4d7e01d
Compare
2 participants
Adds integration test setup with phone emulator, Docker Compose stack, and E2E SMS send/receive tests. CI workflow runs before deployment.